Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: describe OT2 architecture #6245

Merged
merged 15 commits into from
Aug 5, 2020
Merged

Conversation

theosanderson
Copy link
Contributor

@theosanderson theosanderson commented Jul 31, 2020

Overview

This is a quick attempt to describe things that might as an outsider have helped me to understand how my OT2 works. The text is clumsy in many ways and I'm sure not comprehensive, but I thought I'd submit the PR on the basis that something is better than nothing, and as a starting point for improvement. I won't be at all offended if you don't agree and want to hold off on this until you have time to sit down and do it properly.

I think things can be quite opaque e.g. "what does the robot server do?" (#6164 (comment)) and that signposting users to the code that is more likely to be relevant to them (API, then buildroot and robot server) as opposed to not (protocol designer, labware designer, etc.) could be helpful.

Intended to fix (in part) #4375.

@theosanderson theosanderson requested a review from a team as a code owner July 31, 2020 13:48
@codecov
Copy link

codecov bot commented Jul 31, 2020

Codecov Report

❗ No coverage uploaded for pull request base (edge@aa6eba8). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #6245   +/-   ##
=======================================
  Coverage        ?   79.63%           
=======================================
  Files           ?      206           
  Lines           ?    18687           
  Branches        ?        0           
=======================================
  Hits            ?    14881           
  Misses          ?     3806           
  Partials        ?        0           
Impacted Files Coverage Δ
..._server/service/legacy/routers/deck_calibration.py 95.45% <0.00%> (ø)
opentrons/system/log_control.py 52.94% <0.00%> (ø)
opentrons/protocol_api/instrument_context.py 88.54% <0.00%> (ø)
update-server/otupdate/buildroot/__main__.py 0.00% <0.00%> (ø)
opentrons/hardware_control/modules/tempdeck.py 86.98% <0.00%> (ø)
opentrons/drivers/temp_deck/__init__.py 100.00% <0.00%> (ø)
.../service/session/session_types/protocol_session.py 68.96% <0.00%> (ø)
opentrons/drivers/smoothie_drivers/driver_3_0.py 76.00% <0.00%> (ø)
opentrons/util/calibration_functions.py 100.00% <0.00%> (ø)
opentrons/api/util.py 87.50% <0.00%> (ø)
... and 196 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa6eba8...3c5a907. Read the comment docs.

@nickcrider
Copy link
Contributor

Wow, thanks so much for this @theosanderson !! This looks like a pretty good, concise overview of the system architecture to me!

We'll chat about this when everyone is back in the office on Monday and get back to you with some comments. Thanks again for pulling this together. It's been on the list for a long time 😅

@SyntaxColoring SyntaxColoring self-requested a review July 31, 2020 20:11
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple small things to change, but this is a great document and I think a good location for it.

Just to make sure this passes CI, please run make format if you haven't already - we use prettier to (among other things) enforce consistent styles in markdown documents.

OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
@theosanderson
Copy link
Contributor Author

theosanderson commented Aug 3, 2020

Make format is still a todo - will post when done

OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
OT2_ARCHITECTURE.md Outdated Show resolved Hide resolved
@SyntaxColoring
Copy link
Contributor

@theosanderson

  1. As others have said: this is awesome; thank you so much; we love you.

  2. I left lots of little suggestions above.

  3. This is well-timed. I met with @sfoster1 and @amitlissack recently to brainstorm what we would want in a document like this. There's a lot of overlap between what we came up with and what you came up with, but also some differences.

    Here are our notes from that meeting. As an "outsider," do you see anything in this list that strikes you as particularly worthy of inclusion, or not worthy of inclusion?

    Of course, we don't necessarily have to make any more big changes in this PR. If you think something is missing, we can note it and come back to it later.

@theosanderson
Copy link
Contributor Author

theosanderson commented Aug 4, 2020

Hey @SyntaxColoring (and @sfoster1 ) , thanks both so much for all of these great changes. As I said at the top I really wouldn't have minded if you rejected the PR entirely so please don't feel under any pressure to take this over what you were already planning!

It's great to hear that there was already a plan to pull this together. There's lots of great stuff in that Google doc.

I would really like to see here:

  • how to remount the filesystem for read-write access (something I don't know how to do)
  • all the other stuff written from the lens of a Linux user
  • link to HTTP endpoint documentation
  • (as mentioned above my instinct is that documenting, even unofficially and with warnings, a way of connecting the robot by ethernet would be useful)
  • once Eventlog causes hang in build buildroot#87 is addressed I think discussing that additional packages can be selected in buildroot could be good.

I think this PR should now be good for CI. (Let's see). If so I think it could be merged and additional stuff could be part of a future CL when you've had some time to think about details?

@theosanderson
Copy link
Contributor Author

Something else that would be good to document sometime is how calibration works at a high level. My understanding is that deck calibration creates a mapping from deck to smoothie coordinates to adjust for non-squareness, etc. Then pipette calibration makes sure the pipette actually knows where it is in Deck coordinates. Then labware calibration essentially adjusts for errors in the labware definition ? Because labware calibration is not per slot?

@SyntaxColoring SyntaxColoring self-requested a review August 4, 2020 16:15
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Definitely a good start!

@sfoster1
Copy link
Member

sfoster1 commented Aug 4, 2020

Agreed that we should expand it further, but this is absolutely a great start.

@theosanderson
Copy link
Contributor Author

Awesome :) Thanks folks

@shlokamin
Copy link
Member

Coming into this a bit late (sorry!), but when I first joined Opentrons I made a small doc describing the system architecture as I saw it back then. Ping @SyntaxColoring might be helpful for the doc you're making! I'm also happy to help write up anything about the Protocol Designer side if that's in scope for this project.

@mcous mcous changed the title feat(docs): describe OT2 architecture docs: describe OT2 architecture Aug 5, 2020
@mcous mcous added the docs label Aug 5, 2020
@sfoster1 sfoster1 merged commit ea445f0 into Opentrons:edge Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants